Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EHN: FiberBundleNode transmission added #76

Merged
merged 2 commits into from
Oct 17, 2017

Conversation

leochan2009
Copy link
Contributor

@leochan2009 leochan2009 commented Oct 17, 2017

#75

@leochan2009 leochan2009 merged commit be539ca into openigtlink:master Oct 17, 2017
@leochan2009 leochan2009 deleted the Tractography-Transmission branch October 17, 2017 23:07
@jcfr
Copy link
Contributor

jcfr commented Oct 17, 2017

@lassoan @pieper What do you think ? Is it reasonable to integrate this before doing the release ?

@leochan2009 How critical is this patch ? Have you thoroughly tested ? Since we are releasing Slicer 4.8 tonight I am reluctant to integrate this now.

@leochan2009
Copy link
Contributor Author

Hi Jean,

Thanks for the prompt reply.
It is not critical at all, i tested it on my mac only.
I think we could integrate this after the release.

Best,
Longquan

@ihnorton
Copy link
Contributor

@leochan2009 this will work with default output settings/tools, but FiberBundleNode doesn't necessarily require tensors -- UKFTractography allows to disable tensor output. So just be aware that the heuristic will not be 100% accurate.

@lassoan
Copy link
Contributor

lassoan commented Oct 18, 2017

Also, if somebody sends a model node that contains tensor data (for example, stress tensor is computed as a result of finite-element analysis) this code would incorrectly create a fiber bundle node for it.

I would suggest to revert this commit and introduce a different device type for fiber bundle data. You can keep the vtkIGTLToMRMLPolyData class read/write these nodes and just use the device type to indicate that the data should be written to a fiber bundle node.

@leochan2009
Copy link
Contributor Author

Hi Andras,
Great suggestion, currently the mrmlnode created is based on the content of the polydata,
Which is just a quick workaround for the problem of fiberbundle transmission.
I could add a class derived from the igtlPolyDataMessage with deviceType "FiberBundle" into the OpenIGTLink protocol. Also this method could probably solve another problem with color image. #54

@lassoan
Copy link
Contributor

lassoan commented Oct 18, 2017

We could also utilize metadata in OpenIGTLinkv3 - add a custom key/value pair to indicate that the polydata contains fiber bundle (e.g., SUBTYPE=FIBER_BUNDLE).

@tokjun
Copy link
Contributor

tokjun commented Oct 18, 2017

I agree with @lassoan. The OpenIGTLinkIF module should have a mechanism for an OpenIGTLink client (sender) to specify a target MRML type for data conversion in Slicer, because we've seen some use cases where OpenIGTLink messages have to be converted to non-standard types. I would name it MRML_TYPE rather than SUBTYPE.

@leochan2009
Copy link
Contributor Author

Hi All,

I agree with Andras and Junichi to use the the metadata for creating mrmlNode.
This method will solve the mismatch issue between message type and Mrmlnode.
But this solution will create problems when two computer have different version of OpenIGTLink protocol. Set/Get Meta information is only available in recent OpenIGTLink library.
Probably i could add a checkbox in the IF module to send old OpenIGTLink message.

Best,
Longquan

@lassoan
Copy link
Contributor

lassoan commented Oct 18, 2017

Worst that can happen that the old OpenIGTLink client will not get the metadata. I think PLUS toolkit's PlusServer has already a mechanism to determine what version of OpenIGTLink the client can understand and send only compatible messages. This mechanism is implemented is or will be implemented in OpenIGTLinkIO, so when we'll port OpenIGTLinkIF to use OpenIGTLinkIO, it will automatically get this feature.

@adamrankin

@leochan2009
Copy link
Contributor Author

leochan2009 commented Oct 18, 2017

Hi Andras,
I have another OpenIGTLinkIF module that depends on OpenIGTLinkIO library. Will check if the feature is inside.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants